fix(claude): stop counting base64 attachments as raw characters in token estimates - #983
fix(claude): stop counting base64 attachments as raw characters in token estimates#983DevMello wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughClaude request token estimation now treats base64 image and document attachments as bounded data. It uses image dimensions or decoded byte size. Ordinary text remains character-estimated. Routed usage logging and ChangesClaude token estimation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CountTokens as /count_tokens
participant UsageLogging as Routed usage logging
participant Estimator as estimateClaudeRequestTokens
participant ImageSniffer as sniffImageDimensions
participant TextEstimator as estimateTokens
CountTokens->>Estimator: request content and modelId
UsageLogging->>Estimator: system, messages, tools, and modelId
Estimator->>ImageSniffer: inspect base64 image data
ImageSniffer-->>Estimator: image dimensions
Estimator->>TextEstimator: estimate text and adjusted content
TextEstimator-->>Estimator: token estimate
Estimator-->>CountTokens: estimated token count
Estimator-->>UsageLogging: estimated token count
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/claude-messages.ts`:
- Line 869: Update the decoded-size calculation in the attachment token
estimator to remove trailing base64 padding characters before converting encoded
length to bytes, so a padded 131,072-byte payload estimates 256 tokens. Add a
focused regression test alongside the existing attachment-related tests covering
that exact padded payload and expected token count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3c7b18fa-fd1d-4eac-a461-619c523d1e12
📒 Files selected for processing (2)
src/server/claude-messages.tstests/claude-messages-endpoint.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6dc37aee0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (source.type === "base64" && typeof source.data === "string") { | ||
| attachmentTokens += estimateBase64AttachmentTokens(source.data); | ||
| return { ...(entry as Record<string, unknown>), data: "" }; |
There was a problem hiding this comment.
When a Claude history contains an assistant tool_use.input (or another non-attachment JSON object) with fields like {type: "base64", data: "..."}, this branch treats it as an image/document source and removes the string from the estimate. The translator later serializes tool_use.input as function_call arguments, so that base64 text is still sent to routed providers; count_tokens and the cursor/kiro usage-log floor can therefore undercount large tool arguments by orders of magnitude. Limit the special handling to actual Anthropic attachment source objects under image/document blocks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/claude-messages.ts`:
- Around line 892-902: Restrict the attachment-processing logic around the
visible image/document branch to Claude protocol content-block containers,
including nested tool_result.content, and do not recurse into arbitrary
tool_use.input values. Preserve attachment token estimation and data redaction
only for genuine content blocks. Add a focused regression test alongside the
existing claude-messages tests using a complete attachment-shaped object in
tool_use.input and assert it uses the normal serialized-text estimate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d5dd6c5c-bb5c-4215-976e-73d0bf1bcd15
📒 Files selected for processing (2)
src/server/claude-messages.tstests/claude-messages-endpoint.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b78fddf6b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (block.type === "image" || block.type === "document") { | ||
| const source = block.source as { type?: unknown; data?: unknown } | undefined; | ||
| if (source && typeof source === "object" && source.type === "base64" && typeof source.data === "string") { | ||
| attachmentTokens += estimateBase64AttachmentTokens(source.data); |
There was a problem hiding this comment.
With an assistant tool_use.input that contains an object shaped like {type:"image", source:{type:"base64", data:"..."}} (or the same shape inside a tool schema/example), this global replacer still strips the payload and charges the bounded attachment estimate. Those objects are not Anthropic content attachments: assistantMessageToItems serializes tool_use.input into function_call.arguments, and toolsToResponses forwards tool schemas, so the base64 text is still replayed to routed providers. The fresh code still walks the whole messages/tools trees without tracking whether the match is actually inside message/tool_result content, so /count_tokens and the cursor/kiro usage-log floor can undercount large tool arguments by orders of magnitude.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
bun run typecheck && bun run lint:gui && bun run test && bun run privacy:scanChecklist
Summary by CodeRabbit
Improvements
Bug Fixes